Skip to content

Conversation

@VadimCurca
Copy link
Contributor

When both MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS and MLIR multithreading are enabled, topLevelFingerPrint is empty but its value is accessed. This adds a has_value() check before dereferencing the optional.

@VadimCurca VadimCurca marked this pull request as ready for review November 17, 2025 09:27
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Nov 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2025

@llvm/pr-subscribers-mlir-core

Author: Vadim Curcă (VadimCurca)

Changes

When both MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS and MLIR multithreading are enabled, topLevelFingerPrint is empty but its value is accessed. This adds a has_value() check before dereferencing the optional.


Full diff: https://github.com/llvm/llvm-project/pull/168331.diff

1 Files Affected:

  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+1-1)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 365bfddeaab73..b8e78d3287126 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -2766,7 +2766,7 @@ LogicalResult OperationLegalizer::legalizeWithPattern(Operation *op) {
       rewriterImpl.patternMaterializations.clear();
 #if MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS
       // Expensive pattern check that can detect API violations.
-      if (checkOp) {
+      if (checkOp && topLevelFingerPrint.has_value()) {
         OperationFingerPrint fingerPrintAfterPattern(checkOp);
         if (fingerPrintAfterPattern != *topLevelFingerPrint)
           llvm::report_fatal_error("pattern '" + pattern.getDebugName() +

@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2025

@llvm/pr-subscribers-mlir

Author: Vadim Curcă (VadimCurca)

Changes

When both MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS and MLIR multithreading are enabled, topLevelFingerPrint is empty but its value is accessed. This adds a has_value() check before dereferencing the optional.


Full diff: https://github.com/llvm/llvm-project/pull/168331.diff

1 Files Affected:

  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+1-1)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 365bfddeaab73..b8e78d3287126 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -2766,7 +2766,7 @@ LogicalResult OperationLegalizer::legalizeWithPattern(Operation *op) {
       rewriterImpl.patternMaterializations.clear();
 #if MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS
       // Expensive pattern check that can detect API violations.
-      if (checkOp) {
+      if (checkOp && topLevelFingerPrint.has_value()) {
         OperationFingerPrint fingerPrintAfterPattern(checkOp);
         if (fingerPrintAfterPattern != *topLevelFingerPrint)
           llvm::report_fatal_error("pattern '" + pattern.getDebugName() +

@VadimCurca VadimCurca force-pushed the vadimc/fix_optional_access branch from 3ed1138 to c267d3b Compare November 17, 2025 09:32
When both `MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS` and MLIR
multithreading are enabled, `topLevelFingerPrint` is empty but its value
is accessed. This adds a `has_value()` check before dereferencing the
optional.
@VadimCurca VadimCurca force-pushed the vadimc/fix_optional_access branch from c267d3b to 3f19fcc Compare November 17, 2025 09:59
@gysit gysit merged commit e992280 into llvm:main Nov 17, 2025
10 checks passed
aadeshps-mcw pushed a commit to aadeshps-mcw/llvm-project that referenced this pull request Nov 26, 2025
When both `MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS` and MLIR
multithreading are enabled, `topLevelFingerPrint` is empty but its value
is accessed. This adds a `has_value()` check before dereferencing the
optional.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants